Skip to content

Added playwright regression test and fixed more bugs - #7

Merged
fbaltoumas merged 5 commits into
masterfrom
update_architecture
Sep 18, 2026
Merged

fbaltoumas merged 5 commits into
masterfrom
update_architecture

Conversation

@fbaltoumas

Copy link
Copy Markdown
Collaborator

Added a Playwright regression test — and it immediately paid off by catching two more instances of the exact bug class, not just the one I'd already fixed.

The new test (frontend/e2e/smoke.spec.ts, "Kamada-Kawai and Stress layouts run in the background worker on a big network"): posts an 80-node network via /api/external (deterministically over the >60-node worker threshold), runs "kk" then "stress" from the UI, and asserts both
that node positions actually changed and that #layoutStatus is empty (no error). It runs against the real production build, exactly like the rest of this suite (playwright.config.ts's webServer does npm run build && server.py), which is what makes it able to catch
minification-only bugs at all.

What it found: my earlier fix only addressed the dispatcher calling functions by hardcoded name. But getFrWorker()'s pasted function bodies (workStep, frLayout, distanceLayout) also freely reference:

  • the shared S state object's S.workRange (for progress reporting) — nonexistent inside the isolated worker, threw " is not defined" ("d is not defined", "_S is not defined", etc., depending on the build)
  • the module constants FR_SEED and DIST_LAYOUT_LIMIT, referenced in the worker's own hardcoded-name preamble the same fragile way as before

Both are the identical underlying anti-pattern (a function's .toString()'d body, pasted into an isolated worker, referencing something outside its own scope by a name the minifier is free to rename) — my first fix only handled it for functions (via .name); this pass fixes it for
data:

  • workRange/workSink/workLast now live on globalThis.__normaWorkX — globalThis and plain string property names are never renamed, so main thread and worker each get their own correctly-connected copy
  • FR_SEED/DIST_LAYOUT_LIMIT are now inlined as literals (123, 2500) at their two pasted-into-worker use sites, with a comment explaining why, while the real exported constants remain the source of truth for every other (main-thread-only) caller

@vagkaratzas vagkaratzas left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed PR #7 with particular attention to the worker regression. The production build passed; 26/26 Vitest and 12/12 Playwright tests passed, including the new 80-node production-bundle test for kk and stress. The .name-based dispatch matches the minified function names and resolves the reported distanceLayout is not defined error.

One non-blocking follow-up: metrics.ts hard-codes 123 and 2500 in worker-safe code, so future changes to FR_SEED or the layout limit could silently diverge between worker and main-thread behavior. Please consider injecting these values into the worker preamble or passing them explicitly.

@fbaltoumas
fbaltoumas merged commit 5670bbc into master Sep 18, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants